Skip to content

Pin the builder's messages and the version length boundary - #44

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/builder-precondition-tests
Nov 8, 2026
Merged

Pin the builder's messages and the version length boundary#44
dmccoystephenson merged 1 commit into
mainfrom
feature/builder-precondition-tests

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Nov 8, 2026

Copy link
Copy Markdown
Member

Stage B unit-test expansion. Characterization tests are added for the Builder preconditions; no production code is changed.

Summary

  • builder_namesWhatIsMissingInItsMessage: the baseUrl is required, application is required and version is required messages are asserted, along with the order the arguments are checked in (all three missing → baseUrl is required). Before this, only the exception type was checked.
  • builder_rejectsAnOverlongVersionAndAcceptsOneAtTheLimit: a sibling of builder_rejectsAnOverlongInstallIdAndAcceptsOneAtTheLimit. It asserts the version is longer than 255 characters message, that a version of exactly MAX_TAG_LENGTH characters is accepted, and that surrounding whitespace is trimmed before the version is counted and before it is sent as the version tag. The upper side of the boundary was already covered; the at-limit and trimmed sides were not.

Test plan

  • mvn -B verify run locally on JDK 21: 109 tests, 0 failures, 0 errors, 0 skipped. Both new tests appear in the surefire report.
  • CI matrix (Java 8, 17, 21) green on the PR head.

Skipped issues

Every open issue (#13, #15, #27, #31, #39, #40, #42) was left alone. Each one changes the wire format, the opt-out contract or the README "Opting out" wording, so each is waiting on a maintainer decision.

No tracking issue. The gap was found while comparing TraceClient.java with TraceClientTest.java during triage.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson @claude
Pin the builder's messages and the version length boundary
29bfbdf 
The builder's "is required" messages were never asserted, and nothing
showed that a version of exactly MAX_TAG_LENGTH characters is accepted,
or that surrounding whitespace is trimmed before it is counted and sent.
installId already has a test like this; version now has its sibling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

dmccoystephenson commented Nov 8, 2026

Copy link
Copy Markdown
Member Author

Self-review rubric (CI anchor: build (8), build (17), build (21) all green on the PR head):

  • Scope: PASS. git diff --stat origin/main...HEAD shows one file, TraceClientTest.java, +41 lines. No production code, formatting or renames are changed.
  • Tests-new: PASS (not applicable). No public member is added. The two new tests exercise Builder's existing precondition branches.
  • Tests-fix: not applicable. This is a characterization-only Stage B PR with no bug fix. Both tests assert current behaviour and pass on unmodified TraceClient.java.
  • Sibling structure: PASS. builder_rejectsAnOverlongVersionAndAcceptsOneAtTheLimit mirrors builder_rejectsAnOverlongInstallIdAndAcceptsOneAtTheLimit (the same at-limit, overlong and padded cases, plus a message assertion). Both tests use the methodUnderTest_behaviour naming and the Arrange/Act/Assert comments.
  • Sibling renames: not applicable. Nothing is renamed.
  • Docs: PASS. Behaviour is unchanged, so none of the README tables or the Javadoc are affected. The builder(...) Javadoc ("a blank one, or one longer than 255 characters, is an IllegalArgumentException") matches what the new test asserts.
  • Issue resolution: not applicable. The PR has no Closes.
  • CI: PASS. All three matrix legs are green. Locally on JDK 21, mvn -B verify ran 109 tests with 0 failures, and both new tests appear in the surefire report.
  • No dependencies / single file: PASS. pom.xml is unchanged, git ls-files src/main lists only TraceClient.java, and every main import is java.*.
  • Java 8: PASS. The build (8) leg is green, and the new test code uses no post-8 API.
  • Never throws / non-blocking / environment seam / version agreement / opt-out contract: not applicable. TraceClient.java is untouched.

One observation: builder_namesWhatIsMissingInItsMessage pins the order the arguments are checked in (baseUrl first). That order is not documented anywhere, so a maintainer who reorders the checks would need to update this assertion. Pinning it was a deliberate choice, because the message is what a developer sees first.

Summary: a test-only PR with green CI on all three legs and no protected paths touched. It is eligible for merge.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 00b4265 into main Nov 8, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the feature/builder-precondition-tests branch November 8, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dmccoystephenson